Skip to content

Current sensing#11

Open
changxu-liu wants to merge 4 commits into
mainfrom
current_sensor_conversion
Open

Current sensing#11
changxu-liu wants to merge 4 commits into
mainfrom
current_sensor_conversion

Conversation

@changxu-liu
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a driver for the ACS3704x-010B3 current sensor and implements core current sensing functionality for the PDU Mk1, including offset collection and reading routines. Feedback highlights a critical naming inconsistency between the source and header files for the offset variable, which will cause linker errors. Other improvements suggested include correcting copy-pasted documentation, averaging multiple samples for more stable calibration, replacing hardcoded channel mapping with descriptive constants, and investigating a "magic delay" in the test code to resolve potential race conditions.

Comment thread firmware/PowerDistributionUnit_Mk1/core/inc/PDU_Mk1_Current_Sensing.h Outdated
Comment thread firmware/PowerDistributionUnit_Mk1/core/src/PDU_Mk1_Current_Sensing.c Outdated
Comment thread firmware/PowerDistributionUnit_Mk1/test/src/ACS_Current_Sensing.c
@changxu-liu changxu-liu self-assigned this May 13, 2026
Comment on lines +10 to +11
#define ACS3704x_010B3_V_QVO 1.65 // V, quiescent output voltage (no current)
#define ACS3704x_010B3_SENSITIVITY 0.132 // V/A, output voltage increase per amp
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add f to floating point constants to prevent clock cycle waste?

for(uint8_t sample = 0; sample < CURRENT_SENSING_OFFSET_CALC_NUMSAMPLES; sample++)
{
current_adc_v_offset[ch] += -1.0*adc_results_sns1[sample][(ADS131M08Q1_NUM_CHANNELS-1)-ch];
current_adc_v_offset[ADS131M08Q1_NUM_CHANNELS+ch] += -1.0*adc_results_sns0[sample][(ADS131M08Q1_NUM_CHANNELS-1)-ch];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to -1.0f

Copy link
Copy Markdown

@jolynjiang226 jolynjiang226 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

halo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants